Skip to content

chore: Remove Sonar findings#1646

Merged
HofmeisterAn merged 1 commit intodevelopfrom
feature/remove-sonar-smells
Mar 5, 2026
Merged

chore: Remove Sonar findings#1646
HofmeisterAn merged 1 commit intodevelopfrom
feature/remove-sonar-smells

Conversation

@HofmeisterAn
Copy link
Copy Markdown
Collaborator

@HofmeisterAn HofmeisterAn commented Mar 4, 2026 •
edited by coderabbitai bot
Loading

What does this PR do?

-

Why is it important?

-

Related issues

-

Summary by CodeRabbit

  • Bug Fixes

    • Fixed error handling for resource mapping validation.
    • Corrected file structure formatting issues.

  • Refactor

    • Optimized hash computation for different .NET versions.
    • Improved resource disposal handling with conditional compilation support.
    • Enhanced class inheritance constraints for better encapsulation.

@HofmeisterAn HofmeisterAn added the chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups label Mar 4, 2026
@netlify
Copy link
Copy Markdown

netlify bot commented Mar 4, 2026 •
edited
Loading

✅ Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit 809142c
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-dotnet/deploys/69a84ef0624f9f000804bfb8
😎 Deploy Preview https://deploy-preview-1646--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 4, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 00744eeb-1bae-432e-8425-524567966e11

📥 Commits

Reviewing files that changed from the base of the PR and between 934e972 and 809142c.

📒 Files selected for processing (5)
  • src/Testcontainers/Builders/ContainerBuilder3.cs`
  • src/Testcontainers/Builders/DirectoryPath.cs
  • src/Testcontainers/Builders/DockerConfig.cs
  • src/Testcontainers/Builders/FilePath.cs
  • src/Testcontainers/Configurations/WaitStrategies/UntilDatabaseIsAvailable.cs

Walkthrough

These changes comprise bug fixes and framework compatibility improvements across the Testcontainers codebase. Modifications include corrected error handling for URI resource mapping, fixed file structure syntax errors, conditional compilation for framework-specific async disposal patterns, and updated hash computation methodology for Docker context.

Changes

Cohort / File(s) Summary
Path Builders & Resource Mapping
src/Testcontainers/Builders/ContainerBuilder\3.cs, src/Testcontainers/Builders/DirectoryPath.cs, src/Testcontainers/Builders/FilePath.cs`		 Improved error handling for URI-based resource mapping to use argument's Value and Name properties; corrected file structure by removing extraneous closing braces and ensuring proper newline formatting.
Docker Configuration
src/Testcontainers/Builders/DockerConfig.cs		 Updated hash computation to conditionally use Convert.ToHexStringLower() for .NET 10.0+, falling back to existing BitConverter-based approach for earlier framework versions.
Wait Strategies
src/Testcontainers/Configurations/WaitStrategies/UntilDatabaseIsAvailable.cs		 Sealed the class to prevent inheritance; implemented conditional compilation for disposal patterns—synchronous Dispose() for NETSTANDARD2_0, asynchronous DisposeAsync() for other targets.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • testcontainers/testcontainers-dotnet#1497: Modifies the same resource-mapping surface in ContainerBuilder`3.cs and related typed-path handling that this PR refines with improved URI validation and exception construction.

Poem

🐰 With braces now balanced and hashes aligned,
Framework conditions help fix what we find,
Disposal flows smoothly, both sync and async,
The builder hops forward—no need to be drastic!
Small fixes, big wins, in the test-container task! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete with all mandatory sections left empty or marked with '-', providing no explanation of what was changed, why it matters, or how to test the changes. Fill in all mandatory sections: explain the specific Sonar findings addressed, the rationale for each change, link related issues, and provide testing instructions or guidance.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'chore: Remove Sonar findings' is related to the changeset and aligns with the code changes addressing code quality issues, but lacks specificity about which findings are being removed or what the primary change is.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/remove-sonar-smells

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@HofmeisterAn HofmeisterAn marked this pull request as draft March 4, 2026 19:58
@HofmeisterAn HofmeisterAn marked this pull request as ready for review March 5, 2026 18:27
@HofmeisterAn HofmeisterAn merged commit 0a5bb69 into develop Mar 5, 2026
610 of 728 checks passed
@HofmeisterAn HofmeisterAn deleted the feature/remove-sonar-smells branch March 5, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

chore A change that doesn't impact the existing functionality, e.g. internal refactorings or cleanups

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@HofmeisterAn